Add more functions to CompactDecimalFormatter #3699
Conversation
sffc
changed the title
sffc
changed the title
sffc
changed the title
| } | ||
|
|
||
| /// Formats an integer in compact decimal notation using the default | ||
| /// precision settings. |
There was a problem hiding this comment.
Do I understand correctly that if you want something fancier, e.g., at least three digits (like YT subscriber counts), or directed rounding (like everything at YT) you then need to round yourself?
(That works for me.)
There was a problem hiding this comment.
Would you be okay if we landed this PR and then in the future we added such options to CompactDecimalFormatterOptions?
There was a problem hiding this comment.
Sure, though the rounding direction discussion would then come back to haunt us :-)
As I wrote, I am also fine with just not having those options, and seeing the sort of thing that YouTube does as an advanced do-your-own-rounding scenario; since we have format_compact_decimal and CompactDecimal, it is much easier for the caller to have their own arbitrarily fancy rounding logic than it was for me with ICU a few years ago.
| ) -> Result<FormattedCompactDecimal<'_>, CompactDecimalError> { | ||
| use fixed_decimal::FloatPrecision::Floating; | ||
| // NOTE: This first gets the shortest representation of the f64, which | ||
| // manifests as double rounding. |
There was a problem hiding this comment.
I don’t think this is actually a problem given that you do not give the user control over the rounding mode, and that the second rounding is only in the compact case: the two roundings are nearest ties to even, and the ties for the second rounding are representable unless you have really stupidly large numbers (being integers), so the first rounding will not produce a tie for the second.
The double rounding issue would manifest if you could request things such as « nearest, ties to even, exactly one sig. dec. »: then 95.0 / 100.0, which is a little below 0.95, would round to 0.95 in the first rounding, and to 1.0 in the second, whereas it ought to round to 0.9.
|
Scripsit Shane:
No objections from me. |
|
This is mergeable, but I pulled the I decided for now to keep the function named |
(cherry picked from commit cd7d7f9)
|
🎉 All dependencies have been resolved ! |
|
Re-running CI |
| /// The result may have a fractional digit only if it is compact and its | ||
| /// significand is less than 10. Trailing fractional 0s are omitted, and | ||
| /// a sign is shown only for negative values. | ||
| /// |
There was a problem hiding this comment.
This should document that the ryu feature is needed
Part of #3647
Depends on #3945
The concerns about double rounding were discussed in #3647. I'd like to add a function that directly formats a
FixedDecimalbecause otherwise this class is too hard to use.I also made the following changs:
FormattedFixedDecimal::get_compact_decimalto get the result of the compact decimal conversionformat_f64, which is fallible since f64-to-FixedDecimal is fallibleryufeature to the CompactDecimalFormatter crateQuestions:
formatinstead offormat_fixed_decimal?